-
-
Notifications
You must be signed in to change notification settings - Fork 114
Fix logic to retrieve correct line insertion #200
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Previously, function line numbers are extracted from any tag (in any file) so exctracted line is nonsense if the main sketch is empty/has lots of lines and lots of comments/etc Fixes arduino#199
@alto777 @bperrybap could you test with the release produced by @ArduinoBot ? |
✅ Build completed. ⬇️ Build URL: ℹ️ To test this build:
|
I believe I have carefully tested as you request. The OS X arduino-builder placed into the 1.6.13 IDE: The blanks lines no longer cause the problem! |
The linux32 bit version appears to have resolved the issue on Mint 17.1 in IDE version 1.6.13 IDE. |
Sure!
To do so, it scans the tags for the line they are declared in and searches for the smallest of these numbers. Then it searches again, in case a function has been used as a function pointer (thus its prototype need to be declared before this usage). The bug was due to the fact that the search is performed on all sketch files, and the prototypes inserted only in the "main" one. If the main file had less lines than the minimum prototype line number, if was falling back using a safe place, so the bug was invisible. Adding a function to the main file "solves" the issue since it finds a safe hook. |
Speaking of that "not too early", I have seen some issues in some of my stuff where prototypes are generated and the functions have an argument that uses a type that is declared in a library header file but the prototypes are inserted before the include for the header file that declares those types. |
@facchinm, wouldn't it be a better solution to actually insert the prototypes into a secondary file if needed? Or at least at the end of the primary file (IIUC the current code puts them at the top of the primary file now if there are no functions in the primary file?). I recall that the code that handles prototype insertion might also need some refactoring to improve other things, but I can't recall off-hand what those improvements were or where they were discussed. |
@bperrybap, no arduino-builder doesn't actually detect all dependencies between lines of code, it just uses some heuristic (insert just before the first function declaration / definition), combined with very simple dependency checking (to detect a global variable that references a function pointer). In your case, it sounds like it should be a matter of moving the #include for that library upwards, since apparently there is something before it that triggers the protoype insertion already. |
@matthijskooijman |
In your case, the function declaration depends on the include, because it needs the type declarations inside the included header file. To perfectly insert prototypes, these kind of dependencies would need to be known (which isn't possible with the current approach).
Might be useful, yes. It could very well be "intended" (in the sense that it is known limitation of the approach used), but there's also a chance it's a corner case that should be fixed. |
anticipated is would probably a better choice of word than intended. I'll continue this discussion of it elsewhere rather than here since it is a distinctly different issue. |
Just one last followup even though this is off topic for this issue since I didn't want to leave this hanging for this fix/merge. Sorry for the distraction. |
Fixes #199